Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reliably connecting after 1 retry #4

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

billyjacoby
Copy link
Owner

Added proper setup and cleanup of necessary event listeners so to not have to do this every time we run the connect function.

Also ensure that we're creating new components for each individual camera as to not accidentally re-use old RTCPeerConnections or WebSockets.

This seems to make it so that on the second retry attempt we have about 95% successful connection rate. This leads me to believe that we're still not doing something entirely correctly which is what is causing the inconsistency.

This also still only works over a local network connection, which will have to be addressed at some point.

@billyjacoby
Copy link
Owner Author

@domneedham let me know how this works for you!

@billyjacoby billyjacoby force-pushed the fix/setup-and-cleanup-properly branch from a1a20ec to 1dc3212 Compare September 11, 2023 02:32
@domneedham
Copy link
Collaborator

The one camera I have with no audio does seem more responsive so that is an improvement. Still getting the issue:

Simulator Screenshot - iPhone 14 - 2023-09-11 at 08 23 41

I explain the reason for this in #3

@billyjacoby
Copy link
Owner Author

The one camera I have with no audio does seem more responsive so that is an improvement. Still getting the issue:

Simulator Screenshot - iPhone 14 - 2023-09-11 at 08 23 41

I explain the reason for this in #3

Ahh that makes total sense, I was actually looking at that PR incorrectly before. Feel free to rebase that after this goes in and we can get that one in too!

I was doing some more general WebRTC sleuthing late last night and found this post on LogRocket that shows a setup thats pretty different from what we have going on here.

I'm going to try and use some of the code from there tonight to see if I can get more reliable results, and if not I'll spin up an example repo from that code to really dive in a bit more.

Overall we're moving in the right direction and as long as this is sort of working then I'm totally happy to continue building out the rest of the app.

2 similar comments
@billyjacoby
Copy link
Owner Author

The one camera I have with no audio does seem more responsive so that is an improvement. Still getting the issue:

Simulator Screenshot - iPhone 14 - 2023-09-11 at 08 23 41

I explain the reason for this in #3

Ahh that makes total sense, I was actually looking at that PR incorrectly before. Feel free to rebase that after this goes in and we can get that one in too!

I was doing some more general WebRTC sleuthing late last night and found this post on LogRocket that shows a setup thats pretty different from what we have going on here.

I'm going to try and use some of the code from there tonight to see if I can get more reliable results, and if not I'll spin up an example repo from that code to really dive in a bit more.

Overall we're moving in the right direction and as long as this is sort of working then I'm totally happy to continue building out the rest of the app.

@billyjacoby
Copy link
Owner Author

The one camera I have with no audio does seem more responsive so that is an improvement. Still getting the issue:

Simulator Screenshot - iPhone 14 - 2023-09-11 at 08 23 41

I explain the reason for this in #3

Ahh that makes total sense, I was actually looking at that PR incorrectly before. Feel free to rebase that after this goes in and we can get that one in too!

I was doing some more general WebRTC sleuthing late last night and found this post on LogRocket that shows a setup thats pretty different from what we have going on here.

I'm going to try and use some of the code from there tonight to see if I can get more reliable results, and if not I'll spin up an example repo from that code to really dive in a bit more.

Overall we're moving in the right direction and as long as this is sort of working then I'm totally happy to continue building out the rest of the app.

@billyjacoby billyjacoby merged commit 38e0362 into main Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants